Skip to content

Conversation

@jathu
Copy link
Contributor

@jathu jathu commented Jul 11, 2025

Summary

I think EXECUTORCH_BUILD_LLAMA_JNI is redundant. If we are building for Android (assumed true under extension/android) and with EXECUTORCH_BUILD_EXTENSION_LLM, then it implies we want the JNI bindings!

There is a C++ def EXECUTORCH_BUILD_LLAMA_JNI that does some registration:

#ifdef EXECUTORCH_BUILD_LLAMA_JNI
extern void register_natives_for_llm();
#else
// No op if we don't build LLM
void register_natives_for_llm() {}
#endif

However, we turn them on for both BUCK:

non_fbcode_target(_kind = fb_android_cxx_library,
name = "executorch_llama_jni",
srcs = [
"jni_layer.cpp",
"jni_layer_llama.cpp",
"jni_layer_runtime.cpp",
],
allow_jni_merging = False,
compiler_flags = ET_JNI_COMPILER_FLAGS + [
"-DEXECUTORCH_BUILD_LLAMA_JNI",
],

And cmake:

if(EXECUTORCH_BUILD_LLAMA_JNI)
target_sources(executorch_jni PRIVATE jni/jni_layer_llama.cpp jni/log.cpp)
list(APPEND link_libraries llama_runner llava_runner)
target_compile_definitions(executorch_jni PUBLIC EXECUTORCH_BUILD_LLAMA_JNI=1)

Test plan

CI

cc @larryliu0820 @mergennachin @cccclai @helunwencser @jackzhxng

@jathu jathu added ciflow/android Trigger Android CI module: llm Issues related to LLM examples and apps, and to the extensions/llm/ code release notes: none Do not include this in the release notes labels Jul 11, 2025
@pytorch-bot
Copy link

pytorch-bot bot commented Jul 11, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/12382

Note: Links to docs will display an error until the docs builds have been completed.

❌ 7 New Failures, 4 Unrelated Failures

As of commit 4ba9fbb with merge base 9f2b8cd (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 11, 2025
@jathu jathu force-pushed the jathu/no-EXECUTORCH_BUILD_LLAMA_JNI branch from c44e550 to 0ad4a35 Compare July 11, 2025 00:19
@jathu jathu marked this pull request as ready for review July 11, 2025 00:22
@jathu jathu force-pushed the jathu/no-EXECUTORCH_BUILD_LLAMA_JNI branch 3 times, most recently from 3312749 to 7be82a6 Compare July 11, 2025 18:13
Copy link
Member

@GregoryComer GregoryComer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thanks for spotting and cleaning this up.

@jathu jathu force-pushed the jathu/no-EXECUTORCH_BUILD_LLAMA_JNI branch 2 times, most recently from 33f8276 to ebcf4b7 Compare July 11, 2025 19:22
@larryliu0820
Copy link
Contributor

I think this is a good change. cc @kirklandsign

@jathu jathu force-pushed the jathu/no-EXECUTORCH_BUILD_LLAMA_JNI branch 7 times, most recently from c3651af to a4dad94 Compare July 16, 2025 22:29
@jathu jathu force-pushed the jathu/no-EXECUTORCH_BUILD_LLAMA_JNI branch from a4dad94 to 4ba9fbb Compare July 18, 2025 21:01
@jathu jathu closed this Aug 5, 2025
@jathu jathu deleted the jathu/no-EXECUTORCH_BUILD_LLAMA_JNI branch August 5, 2025 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/android Trigger Android CI ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: llm Issues related to LLM examples and apps, and to the extensions/llm/ code release notes: none Do not include this in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants